Skip to content

Conversation

@rolodato
Copy link
Contributor

@rolodato rolodato commented Apr 4, 2025

In #136, I added the TimeSpan Timeout option, with the goal of eventually replacing Double? RequestTimeout. The goal is to accept a TimeSpan and not a number which we then convert to a TimeSpan internally - this is a best practice in .NET and really everywhere I'd say.

After reviewing this in more detail, I decided I don't like the name Timeout because it's vague. I'd really like to keep the name RequestTimeout, but we can't change it to TimeSpan RequestTimeout without making a breaking change.

Instead of deprecating RequestTimeout, I'd prefer to change its type to TimeSpan in 8.x directly. I realise we're not giving advance notice of this breaking change, but at least it's a trivial change:

// before
RequestTimeout = 10

// after
RequestTimeout = TimeSpan.FromSeconds(10)

I only noticed this issue when updating the docs for 7.1.0, which is not yet released.

@rolodato rolodato merged commit 25f657f into main Apr 4, 2025
10 checks passed
@rolodato rolodato deleted the chore/7.1.0 branch April 4, 2025 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants